Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue a warning instead of a syntax error for blif delay constraints #4153

Merged
merged 1 commit into from
Feb 5, 2024
Merged

Issue a warning instead of a syntax error for blif delay constraints #4153

merged 1 commit into from
Feb 5, 2024

Conversation

Coloquinte
Copy link
Contributor

Fix issue #4152

@Coloquinte
Copy link
Contributor Author

I'm getting a CI error with a mismatch on a test (specifically gcc-11 c++-11), but it seems to be already present on master

@mmicko
Copy link
Member

mmicko commented Jan 23, 2024

Please rebase on master. GH actions are fixed meanwhile

@Coloquinte
Copy link
Contributor Author

Please rebase on master. GH actions are fixed meanwhile

It seems that my branch is already based on latest master (2f9fcc2)

@mmicko
Copy link
Member

mmicko commented Jan 23, 2024

Ah. This is since there is no variable defined on your github fork repo. Anyway it is important to run on our CI.

@Coloquinte
Copy link
Contributor Author

Coloquinte commented Jan 23, 2024

The issue persists, and apparently it happens on other pull requests.

@Coloquinte
Copy link
Contributor Author

The Icarus version that is used is the latest (0db1a0c). Given the message (Your branch is up to date with 'origin/master'.), it seems vars.IVERILOG_VERSION is indeed set to master. I don't know which version is supposed to work.

@mmicko
Copy link
Member

mmicko commented Jan 23, 2024

192b6aec96fde982e6ddcb28b346d5893aa8e874 is required version of iverilog

@Coloquinte
Copy link
Contributor Author

Somehow the latest master is pulled. Is the Github variable set to 192b6aec96fde982e6ddcb28b346d5893aa8e874?

@mmicko
Copy link
Member

mmicko commented Jan 23, 2024

Well it looks like variables are not working and are actually taken from PR person context which is not good in this case. Will put version in actions file

@mmicko
Copy link
Member

mmicko commented Jan 23, 2024

Ok done a change, so please just rebase your fork and it should build on your side and as well in PR

@Coloquinte
Copy link
Contributor Author

Very weird behaviour 😮 Thank you for the debugging

@Coloquinte
Copy link
Contributor Author

Hi! Is there any other step for me to do before merging?

@nakengelhardt nakengelhardt merged commit 2422dd6 into YosysHQ:master Feb 5, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants